Skip to content

Conversation

@salvatore-campagna
Copy link
Contributor

@salvatore-campagna salvatore-campagna commented Dec 19, 2024

We experimented with using synthetic source for recovery and observed quite positive impact
on indexing throughput by means of our nightly Rally benchmarks. As a result, here we enable
it by default when synthetic source is used. To be more precise, if index.mapping.source.mode
setting is synthetic we enable recovery source by means of synthetic source.

Moreover, enabling synthetic source recovery is done behind a feature flag. That would allow us
to enable it in snapshot builds which in turn will allow us to see performance results in Rally nightly
benchmarks.

This PR needs a refactoring done in #120096.

Property.Final
);

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is just a result of reordering settings in such a way that static initialization happens in the right order.

@salvatore-campagna salvatore-campagna self-assigned this Jan 10, 2025
@salvatore-campagna salvatore-campagna added the :StorageEngine/Logs You know, for Logs label Jan 10, 2025
tasks.named("yamlRestTest") {
if (buildParams.isSnapshotBuild() == false) {
systemProperty 'tests.rest.blacklist', [
"resources/rest-api-spec/test/60_synthetic_source_recovery.yml"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at other usages, I think this should just be: '60_synthetic_source_recovery/*'?

@salvatore-campagna salvatore-campagna removed the test-release Trigger CI checks against release build label Feb 3, 2025
@salvatore-campagna salvatore-campagna added the test-release Trigger CI checks against release build label Feb 3, 2025
@martijnvg
Copy link
Member

martijnvg commented Feb 4, 2025

Based on #121636, we may not need to feature flag and that will simplify this PR? (never mind)

@salvatore-campagna salvatore-campagna merged commit 6a52675 into elastic:main Feb 5, 2025
18 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 119110

@salvatore-campagna
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

salvatore-campagna added a commit to salvatore-campagna/elasticsearch that referenced this pull request Feb 5, 2025
…ed (elastic#119110)

We experimented with using synthetic source for recovery and observed quite positive impact
on indexing throughput by means of our nightly Rally benchmarks. As a result, here we enable
it by default when synthetic source is used. To be more precise, if `index.mapping.source.mode`
setting is `synthetic` we enable recovery source by means of synthetic source.

Moreover, enabling synthetic source recovery is done behind a feature flag. That would allow us
to enable it in snapshot builds which in turn will allow us to see performance results in Rally nightly
benchmarks.

(cherry picked from commit 6a52675)
salvatore-campagna added a commit that referenced this pull request Feb 6, 2025
… enabled (#119110) (#121760)

* Use synthetic recovery source by default if synthetic source is enabled (#119110)

We experimented with using synthetic source for recovery and observed quite positive impact
on indexing throughput by means of our nightly Rally benchmarks. As a result, here we enable
it by default when synthetic source is used. To be more precise, if `index.mapping.source.mode`
setting is `synthetic` we enable recovery source by means of synthetic source.

Moreover, enabling synthetic source recovery is done behind a feature flag. That would allow us
to enable it in snapshot builds which in turn will allow us to see performance results in Rally nightly
benchmarks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >non-issue :StorageEngine/Logs You know, for Logs Team:StorageEngine test-release Trigger CI checks against release build v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants